Walkthrough

This pull request refactors boolean interval arithmetic constants across the DataFusion codebase. In interval_arithmetic.rs, it renames enum variants CERTAINLY_FALSE to FALSE, CERTAINLY_TRUE to TRUE, and UNCERTAIN to TRUE_OR_FALSE. It introduces additional boolean interval constants TRUE_OR_UNKNOWN, FALSE_OR_UNKNOWN, and ANY_TRUTH_VALUE. Deprecated aliases are added for backward compatibility. Corresponding constants are added to NullableInterval. All usages of renamed variants are updated throughout the codebase in statistics, physical expressions, interval solvers, and join operations. Tests are updated to reflect the new constant names.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Pull Request Review: Rename boolean Interval constants to match NullableInterval

Summary

This PR successfully renames boolean interval constants to use clearer, more semantic names that align with the NullableInterval naming conventions. The changes improve code readability by replacing probabilistic language (CERTAINLY_TRUE, UNCERTAIN) with direct truth value descriptions (TRUE, TRUE_OR_FALSE).

Code Quality & Best Practices ✅

Strengths:

1. Excellent backwards compatibility: The deprecated constants are properly aliased using const assignments, ensuring zero runtime overhead while maintaining API compatibility

   • CERTAINLY_FALSE → FALSE (line 440 in interval_arithmetic.rs)
   • UNCERTAIN → TRUE_OR_FALSE (line 449)
   • CERTAINLY_TRUE → TRUE (line 458)

2. Clear deprecation notices: All deprecated constants include helpful deprecation messages directing users to the new names

3. Comprehensive refactoring: All usages throughout the codebase have been updated consistently across 11 files

4. Improved documentation: New constants include clear doc comments explaining their semantics (e.g., "An interval containing only the 'false' truth value")

5. Semantic alignment: The new NullableInterval::ANY_TRUTH_VALUE constant better expresses its meaning (all three truth values: true, false, unknown) compared to the previous UNCERTAIN

Code Style:

• Follows Rust conventions for deprecation attributes
• Consistent naming patterns across both Interval and NullableInterval types
• Clean, readable test updates that maintain comprehensive coverage

Test Coverage ✅

Excellent test maintenance:

1. All existing tests updated: Tests for and, or, not, comparison operations, and nullable intervals have been properly updated with new constant names

2. Test clarity improved: The new names make test cases more readable:

   (Interval::TRUE, Interval::FALSE, Interval::FALSE)  // Much clearer than
   (Interval::CERTAINLY_TRUE, Interval::CERTAINLY_FALSE, Interval::CERTAINLY_FALSE)

3. Coverage maintained: No test cases were removed; all existing test scenarios continue to validate the interval arithmetic logic

4. Comprehensive test suite: Tests cover:

   • Boolean operations (AND, OR, NOT)
   • Comparison operations (GT, GTE, LT, LTE, EQ)
   • Nullable interval operations
   • Edge cases with null boolean bounds

Potential Issues & Considerations

Minor Observations:

1. Deprecation version (line 439, 448, 457): Uses version "52.0.0"

   • ✅ Appears correct if this is for DataFusion 52.0.0 release
   • Consider: Will deprecated constants be removed in a future version? If so, documenting the removal timeline would help users plan migrations

2. Net line reduction: -468 lines deleted vs +339 added = -129 net lines

   • ✅ This is excellent - the refactoring simplifies the codebase while maintaining functionality

3. No breaking changes for users: Since old constants are deprecated but still work, this is a non-breaking change

   • ✅ Good API evolution practice

Performance Considerations ✅

1. Zero runtime overhead: Using const aliases means no performance impact whatsoever
2. Compile-time substitution: All constant references are resolved at compile time
3. No new allocations: Pure refactoring with identical runtime behavior

Security Concerns ✅

No security concerns identified. This is a pure refactoring that:

• Does not change any logic or algorithms
• Does not introduce new attack vectors
• Does not modify data validation or bounds checking behavior

Specific File Reviews

datafusion/expr-common/src/interval_arithmetic.rs (+253, -688)

Main changes: Constant definitions, usage updates, and test updates

• ✅ Well-structured constant definitions with clear documentation
• ✅ Comprehensive test updates maintain coverage
• ✅ Proper use of deprecation attributes

datafusion/physical-expr/src/expressions/binary.rs (+22, -34)

Main changes: Updated interval constant usage

• ✅ Consistent refactoring of all interval comparisons
• ✅ No logic changes, only naming updates

Other files:

All other changes follow the same pattern - consistent, mechanical refactoring with no logic changes.

Recommendations

Optional Enhancements (not blocking):

1. Consider adding a migration guide: While the deprecation warnings are helpful, a comment in the PR description or commit message about when deprecated constants might be removed would help downstream users

2. Rustdoc example update: Consider adding a code example in the rustdoc comments showing the new constant usage to help users discover the preferred API

Example:

/// An interval containing only the 'true' truth value.
/// 
/// # Example
/// ```
/// use datafusion_expr::interval_arithmetic::Interval;
/// let true_interval = Interval::TRUE;
/// ```
pub const TRUE: Self = ...

Conclusion

Recommendation: APPROVE ✅

This is an excellent refactoring PR that:

• ✅ Improves code clarity and maintainability
• ✅ Maintains full backwards compatibility
• ✅ Has comprehensive test coverage
• ✅ Follows Rust best practices for API evolution
• ✅ Has zero performance impact
• ✅ Contains no security concerns
• ✅ Reduces overall code complexity (-129 lines)

The changes are well-executed, thoroughly tested, and ready to merge. Great work on maintaining API compatibility while improving the codebase!